-
-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for ember test run configuration #166
Conversation
8af0a8f
to
58a1794
Compare
Regarding the dialog I have a few suggestions:
|
and do you think it would be possible to implement the "jump to test code" behavior that is available for other test frameworks? might be something for a future PR |
Grouping only works once the teamcity reporter in testem is updated. see testem/testem#1187
I don't think so. Intellij automatically shows the duration even if it's not provided by the output.
It's possible but requires some additional work. I agree that we should postpone this feature for a future PR. Regarding the form changes, I'll fix that as soon as i have time for it. (Prob later this day) |
58a1794
to
9c89c8a
Compare
I've modified the form based on the issues mentioned above. Sadly the form gets pretty tall like that.
Could you elaborate more on how we can design this in a testem/ember context? |
We don't need to show the "Launchers" list if the radio buttons (mentioned above) are set to "default launchers" (which should obviously be the default) or "no launchers", so in those cases the form is less tall.
similar to:
|
1f5edbb
to
67f66c8
Compare
can you move the separator line of the browsers list above the radio button group?
you can use |
yeah, but I think it could be useful to group the optiongroups and the browserlist together |
67f66c8
to
9914884
Compare
that is very strange. I always use |
If i do it manually i get:
|
ah, maybe that option only works if used with |
We don't support the |
yeah, I guess that makes sense. sorry for the confusion :( |
9914884
to
23496f0
Compare
Ok, I've remove the No Browser option |
23496f0
to
9d3b606
Compare
@makepanic The run configuration seems to assume that the ember module is the root of the project. If it's in a subdirectory you get an error:
|
@dwickern thanks for the feedback, can you check if this error also appears when generating an ember file through the plugin? |
9d3b606
to
f695123
Compare
567ba4e
to
f860ac8
Compare
@makepanic I've just tried this out. A few things that I noticed:
|
also, I've rebased this PR on the current |
f860ac8
to
c0c85ba
Compare
So, stoked for this by the way! |
c0c85ba
to
661e695
Compare
Thanks for rebasing. I've updated the PR so it should fix:
I think the missing error message may be a bug in the teamcity reporter.
compared to tap output:
I'll investigate and maybe create an upstream issue. p.s. i think squash rebasing my changes removed your master rebase information. Meaning the PR only includes changes from me now 🤔 edit: I've created testem/testem#1269 for comparison error results |
This is not about errors in the tests, it's about errors when starting up |
661e695
to
ce2eff3
Compare
ce2eff3
to
5b7a635
Compare
5b7a635
to
73b9e61
Compare
works perfectly now, great work @makepanic! |
@makepanic are there any plans to support server mode, the rebuild every test run makes this quite slow. |
You mean The issue with that is that it spawns a testem instance in development mode with a TUI that we can't hook into. Maybe @johanneswuerbach knows if something like that is possible. I agree with our current setup, it's kinda useless as it will always cause a full build. |
Sorry I step out of frontend development some time ago. Technically it’s possible to run testem dev mode with different reporters (not just the TUI), but I don’t think this is exposed as a flag :-( |
No worries :) Thanks for all the great work you've done with testem. I'll try to see if I can get testem dev work to run with the teamcity reporter in ember-cli. |
@makepanic thanks for all your hard work on this. Looked through the docs and don't see anything obvious about getting different reporters while in dev mode. |
@makepanic that being said it does appear to be supported in the config file. |
See dev_mode_reporter here: https://github.com/testem/testem/blob/master/docs/config_file.md#config-level-options |
So i've hacked around in testem source and overwrote the forced https://github.com/testem/testem/blob/master/lib/config.js#L45 Turns out if we then launch the test run configuration with the server flag, it works out of the box. A potential issue is that intellij adds all tests up, even if rerun, meaning the longer you work the more reported tests you get. If that's something we want to do, i guess the plan would be:
@joegaudet @Turbo87 do you think it's something useful? If so I can start doing above |
@makepanic absolutely, this feature would make ember test writing significantly better. Let me know if there is anything I can do to help. Some additional thoughts.
|
I haven't looked into this. Maybe #109 could be investigated.
I've previously worked on this in 2017 but never finished it. I think we could add this later. |
This PR adds a run configuration that attaches the generated console to intellij
com.intellij.execution.testframework.sm.SMTestRunnerConnectionUtil
which generates the integrated test ui:There isn't any conversion going on because it only pipes testem teamcity output to the test runner.
This means we don't get the most user friendly output right now but once testem/testem#1188 and testem/testem#1187 are resolved, it should display durations and group the tests as seen above.
This PR closes #125
I've added a small paragraph about the test run configuration, without a screenshot, to
features.md
.